Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Allow for inspection of menu state #42

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

danieltwagner
Copy link

I needed these methods in order to connect a fancy rotary input controller to this menu and thought I'd see if you wanted to merge those in. I've attempted to keep the modifications as small as possible while unlocking the functionality I needed.

Taken together, these changes allow me to configure my input controller for the number of items currently on screen as well as the selected item. This works both when a menu is shown or when editing content.

The new methods are:
bool GEM::isEditing() <- whether we're selecting or editing
int GEM::availableOptions() <- the count of options in the current state (e.g. 5)
int GEM::currentOption() <- the (e.g. 2/5)

Further, the GemItems constructors now take an optional pointer to a void enterAction(GemItem *item) callback which allows us to react to the beginning of an edit operation (much like the saveAction allows us to ract to the end of an edit operation).

I think this might make a reasonably unobtrusive addition and would be happy to make modifications you deem necessary. As you see I have not updated any documentation.

@Spirik
Copy link
Owner

Spirik commented Oct 27, 2021

Hello! Can you provide a short snippet here to show how exactly you're using these methods in your sketch?

@danieltwagner
Copy link
Author

danieltwagner commented Oct 27, 2021

Sure thing!
This API is actually not optimal for my own use case. Callbacks would ideally happen in a different order such that I don't have to also track state, but that would have required a larger change to GEM. Hence some "gratuitous" tracking of state below:

GEMItem menuItemPreset(TITLE_PRESET, (int&)preset, myPresets, presetChanged, menuItemSelected);
int savedHapticOptions;
int savedHapticCurrent;
bool invertSelection;

void setup() {
    [...]
    menu.drawMenu();
    savedHapticOptions = menu.availableOptions();
    savedHapticCurrent = menu.currentOption();
    restoreHapticSettings();
}

void rotaryPressed() {
    if(!menu.isEditing()) {
        // we might be about to enter a menu. remember the selected option so we can restore it later
        savedHapticCurrent = menu.currentOption();
    }
    menu.registerKeyPress(GEM_KEY_OK);
}

void rotationChanged(int oldStepPos, int newStepPos) {
    if (newStepPos < oldStepPos) {
        for (int i=0; i < oldStepPos - newStepPos; i++) {
            menu.registerKeyPress(invertSelection ? GEM_KEY_UP: GEM_KEY_DOWN);
        }
    } else if (newStepPos > oldStepPos) {
        for (int i=0; i < newStepPos - oldStepPos; i++) {
            menu.registerKeyPress(invertSelection ? GEM_KEY_DOWN : GEM_KEY_UP);
        }
    }
}

void presetChanged() {
    // about to leave edit sequence, restore previous haptics
    restoreHapticSettings();
    [...]
}

void restoreHapticSettings() {
    invertSelection = true;
    idrive.configureHapticFeedback(savedHapticOptions, savedHapticCurrent);
}

void menuItemSelected(GEMItem *item) {
    // about to start editing. configure the correct haptics
    // invert selection for the preset menu to keep natural scroll direction
    invertSelection = (strcmp(item->getTitle(), TITLE_PRESET) == 0);
    idrive.configureHapticFeedback(menu.availableOptions(), menu.currentOption());
}

@Spirik
Copy link
Owner

Spirik commented Oct 27, 2021

Apologies for not being too quick to response. I'm having rather busy week. Hopefully will be able to have a closer look at the weekend. Meanwhile I may ask you some more about incentive behind these updates=)

At a quick glance it seems that requirement to know current item index (as well as a total available number of options at the current mode) is rather specific for the rotary encoder/library you are using. I need to think more whether it can be useful in other cases. I was having some thoughts on how to expose some of the inner state of the menu to the sketch anyway, so that may be a possible place to start.

Do that values allow you to set up your encoder for a proper acceleration/rate at which steps are performed (in idrive.configureHapticFeedback method), or am I misunderstand something?

Does navigation from one digit to another (I mean horizontal cursor position) still require additional left and right buttons, separate from rotary encoder?

@danieltwagner
Copy link
Author

No rush at all.

Do that values allow you to set up your encoder for a proper acceleration/rate at which steps are performed (in idrive.configureHapticFeedback method), or am I misunderstand something?

It's a rotary encoder with programmable haptic feedback, so I use this to tell the controller that e.g. there should be 5 indents with hard stops afterwards, and that the currently selected index should be 2/5. The specifics are very much to do with the particular controller.

I was having some thoughts on how to expose some of the inner state of the menu to the sketch anyway, so that may be a possible place to start.

I did push a couple of other attempts to Github with earlier, more invasive, ideas for how to make state accessible. I didn't like those as much but you could have a look at it: https://github.com/danieltwagner/GEM/tree/item-inspection

Does navigation from one digit to another (I mean horizontal cursor position) still require additional left and right buttons, separate from rotary encoder?

Yes, this PR does not change the interaction with the menu at all. It only allows me to inspect state so that I can configure my controller.

@Spirik
Copy link
Owner

Spirik commented Nov 1, 2021

I am thinking of may be adding a special GEM::state property in GEM object, which will contain a certain information regarding current state of the menu object (e.g. current GEMItem object, GEMPage object, mode, indexes, etc.). To get this information user would access fields and methods of the said state property, e.g. myMenu.state.getCurrentMenuItemIndex(), myMenu.state.isEditing, etc.

That way it is possible to make the state property optional and configurable (via settings.h and compiler flags) to save space for users that do not need this additional functionality. The space for adding new features is of a big concern currently, because Adafruit GFX version of Party Hard example already takes 99% of Arduino UNO program space. Also it will be easier to extend functionality of the state object in the future without crowding name space of the GEM object itself.

I will need to think a little more in that direction.

@danieltwagner
Copy link
Author

That makes a lot of sense. I was not space constrained so didn't optimize for it. Good to hear you're thinking about exposing some state! One thing I'd mention is that I find that a "natural" scroll direction differs between menu and editing mode, so that is one thing I use the state for (swapping up/down).

@Spirik
Copy link
Owner

Spirik commented Sep 28, 2023

Bump this lonely PR a little=)

I finally got around to do some experiments with "clicky" rotary encoder (with built-in push-button). Swapping direction of GEM_KEY_UP and GEM_KEY_DOWN in edit mode turned out to be really important for more natural and expected behavior indeed.

So I've added ::invertKeysDuringEdit() method to do just that. Call it once (e.g. in setup()) and up/down directions will be swapped when navigating through characters of a single digit (of char[17] or number variable).

I've created detailed example on how to operate menu with rotary encoder for each of three versions of GEM (e.g. U8g2 version) using latest version of KeyDetector library.

Adding some kind of State object is still very much on my radar, it is a requested feature.

@Spirik Spirik mentioned this pull request Oct 4, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants